Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #2878: apoc 4.4.0.4 is reported as incompatible with neo4j-community-4.4.6 in neo4j.log #2885

Merged
merged 2 commits into from
May 20, 2022

Conversation

vga91
Copy link
Collaborator

@vga91 vga91 commented May 13, 2022

Fixes #2878

@vga91
Copy link
Collaborator Author

vga91 commented May 13, 2022

@Lojjs Sorry about this annoying bug. Unluckily I tried it on a version where it worked 🤦.
Please, can you (or someone else of surface) review it?

Copy link
Contributor

@Lojjs Lojjs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This definitely looks more correct, but I wonder could we be certain that both Neo4j and APOC always will have the versioning separator .?

If yes, I guess we can simplify and just hard-code it.
If no, we can do the comparison independent of the separator so that e.g. 4_4_5 is shown as compatible with 4.4.0.4 based on that the first two numbers are the same.
What do you think?

@Lojjs Lojjs self-assigned this May 13, 2022
@vga91 vga91 force-pushed the issue-2878 branch 3 times, most recently from be10e7a to b08de7f Compare May 13, 2022 12:18
@vga91
Copy link
Collaborator Author

vga91 commented May 13, 2022

This definitely looks more correct, but I wonder could we be certain that both Neo4j and APOC always will have the versioning separator .?

If yes, I guess we can simplify and just hard-code it. If no, we can do the comparison independent of the separator so that e.g. 4_4_5 is shown as compatible with 4.4.0.4 based on that the first two numbers are the same. What do you think?

Yes, the second option (4_4_5 = 4.4.0.4) probably makes more sense.
I just changed it in this way

Copy link
Contributor

@Lojjs Lojjs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@vga91 vga91 merged commit 390d4a0 into neo4j-contrib:4.4 May 20, 2022
vga91 added a commit to vga91/neo4j-apoc-procedures that referenced this pull request May 20, 2022
vga91 added a commit to vga91/neo4j-apoc-procedures that referenced this pull request May 24, 2022
vga91 added a commit to vga91/neo4j-apoc-procedures that referenced this pull request May 25, 2022
vga91 added a commit to vga91/neo4j-apoc-procedures that referenced this pull request May 25, 2022
vga91 added a commit to vga91/neo4j-apoc-procedures that referenced this pull request May 25, 2022
vga91 added a commit to vga91/neo4j-apoc-procedures that referenced this pull request May 26, 2022
vga91 added a commit that referenced this pull request May 27, 2022
vga91 added a commit to vga91/neo4j-apoc-procedures that referenced this pull request May 27, 2022
vga91 added a commit that referenced this pull request May 27, 2022
vga91 added a commit that referenced this pull request May 30, 2022
@vga91
Copy link
Collaborator Author

vga91 commented May 31, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

apoc 4.4.0.4 is reported as incompatible with neo4j-community-4.4.6 in neo4j.log
2 participants